Improve "Name is not defined" errors with fuzzy matching#20693
Improve "Name is not defined" errors with fuzzy matching#20693KevinRK29 wants to merge 5 commits intopython:masterfrom
Conversation
This comment has been minimized.
This comment has been minimized.
KevinRK29
force-pushed
the
add-fuzzy-matching
branch
from
ac38021 to
31d0bd2
Compare
KevinRK29
force-pushed
the
add-fuzzy-matching
branch
from
54f3d59 to
55a8efc
Compare
for more information, see https://pre-commit.ci
|
Diff from mypy_primer, showing the effect of this PR on open source code: manticore (https://github.com/trailofbits/manticore)
- tests/auto_generators/make_dump.py:390: error: Name "xrange" is not defined [name-defined]
+ tests/auto_generators/make_dump.py:390: error: Name "xrange" is not defined; did you mean "range"? [name-defined]
apprise (https://github.com/caronc/apprise)
- apprise/plugins/irc/client.py:115: error: Name "Deque" is not defined [name-defined]
+ apprise/plugins/irc/client.py:115: error: Name "Deque" is not defined; did you mean "deque"? [name-defined]
AutoSplit (https://github.com/Toufool/AutoSplit)
- src/utils.py:178:20: error: Name "FilterGraph" is not defined [name-defined]
+ src/utils.py:178:20: error: Name "FilterGraph" is not defined; did you mean "filter_graph"? [name-defined]
- src/utils.py:184:12: error: Name "COMError" is not defined [name-defined]
+ src/utils.py:184:12: error: Name "COMError" is not defined; did you mean "IOError" or "OSError"? [name-defined]
|
| @@ -608,7 +608,7 @@ def f(x: _T) -> None: pass | |||
| s: FrozenSet | |||
| [out] | |||
| _program.py:2: error: Name "_T" is not defined | |||
| _program.py:3: error: Name "FrozenSet" is not defined | |||
| _program.py:3: error: Name "FrozenSet" is not defined; did you mean "frozenset"? | |||
There was a problem hiding this comment.
Can you add an explicit test in some check-*.test file that ensures that misspellings of builtins are detected?
|
|
||
| y = MyClas() # E: Name "MyClas" is not defined; did you mean "MyClass"? [name-defined] | ||
|
|
||
| unknown_xyz # E: Name "unknown_xyz" is not defined [name-defined] |
There was a problem hiding this comment.
Ideas for additional tests:
- Test a misspelling of a local variable.
- Test a mispelling of a top-level definition within a function (misspelling targeting an outer namespace).
- Test a misspelling of a class variable within class body.
- Test a misspelling of
namethat is available throughfrom import name.
| and not (name.startswith("__") and name.endswith("__")) | ||
| and f"builtins.{name}" not in SUGGESTED_TEST_FIXTURES | ||
| ): | ||
| alternatives = self._get_names_in_scope() |
There was a problem hiding this comment.
Since this is pretty expensive, we shouldn't do the analysis if the error will just be ignored. This way we'd only slow things down when there is a user-visible error. In some codebases, there could be thousands of ignored errors, and this could generate non-trivial amounts of extra work.
My suggestion would be to refactor is_ignored_error in mypy/errors.py so that most of the checking (but not the blocker check) is available through is_ignored_error_code(line, code, ignores), for example. Then you can check if codes.NAME_DEFINED is ignored here, and skip the expensive logic.
There was a problem hiding this comment.
Actually, it would be fine to skip the logic if there is any ignore comment on the line, if that's easier to implement.
| names.update(table.keys()) | ||
|
|
||
| if self.type is not None: | ||
| names.update(self.type.names.keys()) |
There was a problem hiding this comment.
Class namespace should only be included if we are not within a method (i.e. directly within class body only).
2 participants
This PR collects all visible names in the current scope and uses fuzzy matching to provide suggestions in the error messages